Remove default implementation for the DecCBOR class#5532
Conversation
libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/DecCBOR.hs
Show resolved
Hide resolved
lehins
left a comment
lehins
left a comment
There was a problem hiding this comment.
There is really no need for all of those extra top level definitions for reusability in both types of decoders, they only bring noise.
It is better to use special helper fromEraCBOR (see my suggestion)
We could even potentially add a newtype like this to Cardano.Ledger.Core.Era:
newtype EraDecoder era a = EraDecoder a
instance (Era era, DecCBOR a) => FromCBOR (EraDecoder era) where
fromCBOR = fromEraCBOR @era
which can then be used to do deriving via.
| decCBOR = do | ||
| sbs <- decCBOR @ShortByteString | ||
| maybe (fail "Invalid hash size") pure $ hashFromBytesShort sbs | ||
| {-# INLINE decCBOR #-} |
There was a problem hiding this comment.
We actually need to create a DecCBOR instance for PackedBytes. For now that instance will simply biggy back on FromCBOR instance, but then we will need to export some functionality for safe construction of PackedBytes from this module: `https://github.com/IntersectMBO/cardano-base/blob/d78f2f350017f0b3a1c57f483a2c323676756622/cardano-crypto-class/src/Cardano/Crypto/PackedBytes.hs
| decCBOR = do | |
| sbs <- decCBOR @ShortByteString | |
| maybe (fail "Invalid hash size") pure $ hashFromBytesShort sbs | |
| {-# INLINE decCBOR #-} | |
| decCBOR = hashFromPackedBytes <$> decCBOR | |
| {-# INLINE decCBOR #-} |
There was a problem hiding this comment.
Ah I see - that's a hidden module so even the FromCBOR instance is not exported. I guess this is blocked on exposing at least that from base?
libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/DecCBOR.hs
Show resolved
Hide resolved
f-f
force-pushed
the
f-f/no-default-deccbor
branch
from
c7f8ec1 to
277ebdb
Compare
2 participants
Description
First part of #5513: we remove the default implementation for
DecCBORthroughFromCBOR, so that types that contain Bytestrings are more easily routed to the version-aware decodingChecklist
CHANGELOG.mdfiles updated for packages with externally visible changes.NOTE: New section is never added with the code changes. (See RELEASING.md).
.cabalandCHANGELOG.mdfiles when necessary, according to theversioning process.
.cabalfiles updated when necessary.NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
scripts/fourmolize.sh).scripts/cabal-format.sh).scripts/gen-cddl.sh)hie.yamlupdated (usescripts/gen-hie.sh).